Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix apply on user-defined aggregate init #2456

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

quentin
Copy link
Member

@quentin quentin commented Dec 18, 2023

The init expression of user-define aggregates was never handed-over to apply's mapper.

One visible issue is that the TupleId transformer would not update the tuple of the init expression while shuffling tuple identifiers. That would then result in the init expression peeking data in the wrong tuple during execution of the RAM program.

The `init` expression of user-define aggregates was never
handed-over to `apply`'s `mapper`.

One visible issue is that the `TupleId` transformer would not update the
tuple of the `init` expression while shuffling tuple identifiers.
That would then result in the `init` expression peeking data in the
wrong tuple during execution of the RAM program.
@quentin quentin requested a review from julienhenry December 18, 2023 14:28
Copy link

codecov bot commented Dec 18, 2023

Codecov Report

Merging #2456 (abcaad7) into master (2d20ade) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2456      +/-   ##
==========================================
- Coverage   78.19%   78.15%   -0.04%     
==========================================
  Files         486      486              
  Lines       32374    32379       +5     
==========================================
- Hits        25315    25306       -9     
- Misses       7059     7073      +14     
Files Coverage Δ
src/ast/UserDefinedAggregator.cpp 96.29% <100.00%> (-0.14%) ⬇️
src/ram/Aggregate.h 67.50% <100.00%> (+0.83%) ⬆️
src/ram/Aggregator.h 60.00% <100.00%> (+4.44%) ⬆️
src/ram/IndexAggregate.h 69.56% <100.00%> (+0.67%) ⬆️
src/ram/UserDefinedAggregator.h 78.57% <100.00%> (+2.57%) ⬆️

... and 5 files with indirect coverage changes

@quentin quentin merged commit f766700 into souffle-lang:master Dec 19, 2023
30 checks passed
quentin added a commit to quentin/souffle that referenced this pull request Jun 19, 2024
The `init` expression of user-define aggregates was never
handed-over to `apply`'s `mapper`.

One visible issue is that the `TupleId` transformer would not update the
tuple of the `init` expression while shuffling tuple identifiers.
That would then result in the `init` expression peeking data in the
wrong tuple during execution of the RAM program.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants